fix(registry): label collision versions meaningfully#2792
Conversation
|
✅ No security or compliance issues detected. Reviewed everything up to da622e6. Security Overview
Detected Code Changes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc19c6c118
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| suffix = datetime.now(UTC).strftime("%Y%m%d%H%M%S%f") | ||
| tiebreaker = cast(int, uuid.uuid4().int) % 1_000_000 | ||
| return f"{base_version}.dev{suffix}{tiebreaker:06d}" | ||
| return f"{base_version}.post{suffix}+collision.{tiebreaker:06d}" |
There was a problem hiding this comment.
Preserve valid collision labels for post releases
When the base registry version is itself a supported post release such as 1.2.3-post.1 (scripts/update-version.sh accepts post in PUBLIC_SUFFIX_PATTERN), this produces 1.2.3-post.1.post...+collision..., which packaging.version.Version rejects. If that collision becomes the current platform registry version, the next startup downgrade check in tracecat/registry/sync/jobs.py::_is_downgrade falls into _release_tag_key, which also cannot parse the collision suffix, so startup registry sync aborts instead of comparing versions. The previous .dev... suffix remained PEP 440-parseable for post releases, so this is a regression for that release channel.
Useful? React with 👍 / 👎.
### Motivation - Avoid using the ambiguous `.dev` suffix for registry-version collisions and instead emit a more descriptive, compliant post-release label that signals a collision while remaining sortable and SemVer-compatible where possible. ### Description - Change collision version generation in `tracecat/registry/sync/base_service.py` by replacing the `.dev<ts><tiebreaker>` scheme with `.post<timestamp>+collision.<tiebreaker>` via `_generate_collision_version`. - Update unit tests in `tests/unit/test_registry_sync_base_service.py` to assert the new `.post...+collision.` format and ensure the collision marker exists in the generated version string. - Update platform-startup fixtures and expectations in `tests/unit/test_registry_platform_startup.py` to use the new collision version format for scheduled/promoted collision versions. ### Testing - Ran linting with `uv run ruff check tracecat/registry/sync/base_service.py tests/unit/test_registry_sync_base_service.py tests/unit/test_registry_platform_startup.py` which passed successfully. - Verified `git diff --check` to ensure no whitespace/format issues and it passed. - Attempted to run the affected unit tests with `uv run pytest` for targeted tests (`test_sync_creates_collision_version_for_manifest_changes`, `test_startup_sync_schedules_artifact_for_resolved_version`, `test_promote_after_artifact_build_promotes_collision_version`) but the test run errored due to a local PostgreSQL connection failure on `localhost:5432` (database was not available), so full pytest validation is blocked until a test DB is available.
Motivation
.devsuffix for registry-version collisions and instead emit a more descriptive, compliant post-release label that signals a collision while remaining sortable and SemVer-compatible where possible.Description
tracecat/registry/sync/base_service.pyby replacing the.dev<ts><tiebreaker>scheme with.post<timestamp>+collision.<tiebreaker>via_generate_collision_version.tests/unit/test_registry_sync_base_service.pyto assert the new.post...+collision.format and ensure the collision marker exists in the generated version string.tests/unit/test_registry_platform_startup.pyto use the new collision version format for scheduled/promoted collision versions.Testing
uv run ruff check tracecat/registry/sync/base_service.py tests/unit/test_registry_sync_base_service.py tests/unit/test_registry_platform_startup.pywhich passed successfully.git diff --checkto ensure no whitespace/format issues and it passed.uv run pytestfor targeted tests (test_sync_creates_collision_version_for_manifest_changes,test_startup_sync_schedules_artifact_for_resolved_version,test_promote_after_artifact_build_promotes_collision_version) but the test run errored due to a local PostgreSQL connection failure onlocalhost:5432(database was not available), so full pytest validation is blocked until a test DB is available.Codex Task
Summary by cubic
Replace ambiguous
.devcollision versions with a local label+collision.<timestamp>.<tiebreaker>(or.collision...when the base already has local metadata) to make collisions explicit while preserving the base release and keeping versions sortable and SemVer-friendly.Updated unit tests and platform startup expectations, and added coverage to ensure pre-release, post-release, and existing local suffixes are preserved.
Written for commit da622e6. Summary will update on new commits.